-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[core] shader program must always match bucket in render symbol layer #13667
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem with storing the paint property values in the bucket is that if you modify just a paint property but no layout properties, the bucket doesn't get re-generated -- so with this code, your paint property change wouldn't show up until something else triggered a tile reload. This is why I tried to save just the constant Bitset (for shader selection) instead of the values themselves in the bucket-bitset
branch -- although it's somewhat more complicated.
On top of the "paint changes, layout doesn't" case, there's also the case where someone changes both paint and layout properties at the same time, triggering re-layout. The current (and intended) behavior is that the paint property change takes effect immediately, while the layout change doesn't take effect until the tile reloads. You could definitely argue that it would be more consistent for both changes to take place at the same time, but I think so far we've prioritized making constant-value paint property changes instantaneous.
Also, however we fix this, we should do it across all the layer types, right?
f2817a6
to
01356b5
Compare
@ChrisLoer Thanks for your comments! Paint properties are now updated for the complete tile buckets. PTAL. |
Sure thing! Once we agree on the approach I'd be happy to spread it to other layer types. |
Before this change, `RenderSymbolLayer` with updated style was trying to render symbols using the previous bucket (with paint property binders that matched a previous program). Now, symbol bucket caches the latest corresponding paint properties (caching is happening on complete tiles only). As a result, `RenderSymbolLayer` always picks the shader program and its parameters in sync with the obtained bucket.
01356b5
to
e01182c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
Before this change,
RenderSymbolLayer
with updated style was trying to rendersymbols using the previous bucket (with paint property binders that matched a previous program).
Now, symbol bucket caches the latest corresponding paint properties (caching is happening on
complete tiles only). As a result,
RenderSymbolLayer
always picks the shader program andits parameters in sync with the obtained bucket.
Fixes: #13407